-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add force_void instruction #239
feat: add force_void instruction #239
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A comment for consideration
require!( | ||
matching_queue.is_empty(), | ||
CoreError::CloseAccountMarketMatchingQueueNotEmpty | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this what we talked through, and if we get to this point we should be safe? Since you can only close something voided or settled and can only go to those states if the queues are empty?
I had momentarily forgotten that, and was about to suggest that ForceVoidMarket take the matching queue but rather than check that it is empty, just clear it and set len to 0. So we could leave this check here.
I think if we're safe at this point it's OK to remove from here. Though somewhat inconsistent keeping order request len check but not matching queue len check.
From a hotfix pov, I do like the idea of the new instruction just clearing the matching queue, so we don't have code changes outside the new instructions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do like the idea of the new instruction just clearing the matching queue, so we don't have code changes outside the new instructions.
Happy to do this, what I have done is safe but yes containing hotfix changes to the new instruction is probably better
Add an instruction which will allow markets to be voided when items still remain in the matching queue.
Tests: